Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improvement of auto-escaping #1030

Merged
merged 5 commits into from
Jun 30, 2024
Merged

Improvement of auto-escaping #1030

merged 5 commits into from
Jun 30, 2024

Conversation

Amaury
Copy link
Contributor

@Amaury Amaury commented Jun 9, 2024

This evolution improves the auto-escaping feature.

  • The escape modifier has no effect when auto-escaping is enabled (when no escape format is given, or when the html format is used), to prevent double-escaping.
  • The other formats of the escape modifier (htmlall, url, urlpathinfo, quotes, javascript) are processed as we may expect, without double-escaping.
  • The new force format of the escape modifier allows to force double-escaping if needed.
  • The new raw modifier temporary disables auto-escaping for the expression it is used on.

This Pull Request contains the source code of this evolution, as well as its documentation and unit tests.

Amaury added 2 commits June 9, 2024 11:16
…' modifier; add the 'force' mode to the 'escape' modifier; add the 'raw' modifier.
@wisskid
Copy link
Member

wisskid commented Jun 19, 2024

This is looking great, @Amaury !

How do you propose we release this? Do you think this calls for a new major version, i.e. Smarty v6 or a minor (v5.4)?

One might argue this requires a major, because behavior is changing for existing templates that use |escape of some form when html auto-escaping is also enabled. On the other hand: that would very likely be unintended anyway.

@Amaury
Copy link
Contributor Author

Amaury commented Jun 19, 2024

About the release, I would say that a minor version should be enough; I don't know if anybody if really using auto-escaping and |escape at the same time. On the other hand, it's true this is a deep change of behaviour…
I don't know, I let you choose ^^'

@wisskid wisskid self-assigned this Jun 30, 2024
@wisskid wisskid merged commit 2289fa6 into smarty-php:master Jun 30, 2024
11 checks passed
@Amaury
Copy link
Contributor Author

Amaury commented Jul 11, 2024

Thank you! :)

@Amaury
Copy link
Contributor Author

Amaury commented Jul 11, 2024

In the end, do you think you will create a major version or a minor one?

@wisskid
Copy link
Member

wisskid commented Jul 11, 2024

I'm thinking minor.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants